Use external distribution of Juliet test suite #155
Conversation
|
As far as I can tell, all failing tests are CWE89, which are NOT present in |
The failing tests are due to defects in the analyzer for SQL injections, I'll create a separate issue for this. Indeed, I haven't tested analyzer on them previously (both for simplicity and saving CI time), let's just ban them for now. |
Codecov Report
@@ Coverage Diff @@
## develop #155 +/- ##
==========================================
Coverage 77.09% 77.09%
Complexity 1520 1520
==========================================
Files 157 157
Lines 8630 8630
Branches 1484 1484
==========================================
Hits 6653 6653
Misses 1413 1413
Partials 564 564 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
jacodb-analysis/build.gradle.kts
Outdated
| @@ -6,20 +6,26 @@ plugins { | |||
| kotlin("plugin.serialization") version "1.7.20" | |||
| } | |||
|
|
|||
| repositories { | |||
There was a problem hiding this comment.
Please move this to the root build.gradle.kts
There was a problem hiding this comment.
Why? Jitpack repo is only needed for this subproject.
Is it just to make it easier to keep track of external repos from one place (root build)?
There was a problem hiding this comment.
It's a good practice to declare global settings in global place. Even if this settings is used in one submodule.
| testImplementation(files("src/test/resources/pointerbench.jar")) | ||
| testImplementation(group = "joda-time", name = "joda-time", version = "2.12.5") | ||
| testImplementation("com.github.Lipen.juliet-test-suite:support:1.3.6") | ||
| for (cweNum in listOf(89, 476, 563, 690)) { |
There was a problem hiding this comment.
It's better to store such settings in gradle.properties
There was a problem hiding this comment.
Currently, only these CWEs are supported by an existing analysis (afaik), so we only depend on them, but in the future the list will probably grow and might even be replaced with a whole suite.
Gradle properties are just Strings, aren't they? The simple listOf in dependencies {} seems to be cleaner than parsing [89, 476, 563, 690] string into a list of ints.
jacodb-analysis/build.gradle.kts
Outdated
| testImplementation(files("src/test/resources/pointerbench.jar")) | ||
| testImplementation(group = "joda-time", name = "joda-time", version = "2.12.5") | ||
| testImplementation("com.github.Lipen.juliet-test-suite:support:1.3.6") |
There was a problem hiding this comment.
There is a license problem with this dependency. Repo itself doesn't declare any license but have a reference that it is a copy of NIST lib which is under CC 1.0 into README.
I prefer to reference NIST Juliet itself (i.e remove any reference to find-sec-bugs repo) from your repo. Also it's better to host this repo inside UTBot organization.
There was a problem hiding this comment.
The ZIP archive from NIST does not contain the LICENSE file, either. Should I simply create one (CC0)?
Regarding the repo: I can re-upload the project under UTBot org, without "forking" (I bet I cannot simply "Transfer repo", since it would probably remain to be "forked"), but what about the git history? The initial commits were made by find-sec-bugs's members.
There was a problem hiding this comment.
I've moved Juliet repo to https://github.com/UnitTestBot/juliet-java-test-suite
|
@Lipen could you please resolve conflicts |
This PR proposes to change the vendored Juliet test suite (pre-compiled JAR manually placed in
resourcesfolder) with an external distribution via JitPack.